fix(monitor): cap Span.Progress so a Running zero-total child cannot push count past total#1249
Open
SAY-5 wants to merge 1 commit intogorse-io:masterfrom
Open
Conversation
…st parentTotal Span.Progress treated every zero-total child as fully complete by adding childTotal to parentCount. Combined with the parent contributing its own count*childTotal, this produced parentCount > parentTotal whenever the parent had already finished its own work and a Running zero-total child was still registered. The dashboard then rendered the task at >100%. A Running zero-total child has no measurable progress yet; once it completes, it is filtered out of the children loop entirely and the parent's own count covers it via the s.count * childTotal term. So the right thing to do is contribute 0 for any zero-total child still in the loop, rather than treating its empty work as a finished branch. Update the existing 'divide by zero' / 'child with zero total' / 'all children with zero total' tests to reflect the corrected accounting (Running zero-total contributes 0, not childTotal), and add a focused regression for the parent-finished-with-running-child scenario from the report. The new assertion fails on master with 'Count (3) must not exceed Total (2)'. Closes gorse-io#1241 Signed-off-by: SAY-5 <SAY-5@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes #1241.
Span.Progresswas treating every zero-total child as fully complete by addingchildTotaltoparentCount. Combined with the parent's owncount*childTotalterm, this producedparentCount > parentTotalwhenever the parent had already finished its own work and a Running zero-total child was still registered. The dashboard then rendered the task at >100%.Repro (from the issue)
Parent
total=2, count=2, one Running zero-total child:s.count * childTotalProgress{Count: 3, Total: 2}(150%)Fix
A Running zero-total child has no measurable progress yet. Once it completes, it is filtered out of the children loop entirely (the loop only includes
StatusRunning) and the parent's own count covers it through thes.count * childTotalterm. So the right thing to do is contribute 0 for any zero-total child that is still in the loop, rather than treating its empty work as a finished branch.Tests
divide by zero/child with zero total/all children with zero totalcases to reflect the corrected accounting (Running zero-total contributes 0, notchildTotal).zero-total running child does not exceed parent totalas the focused regression for the report. On master it fails withCount (3) must not exceed Total (2); with the fix it passes.Verification
Locally on macOS, go 1.26.2:
gofmt -s -l common/monitor/: cleango vet ./common/monitor/...: cleango test -race -count=1 ./common/monitor/...: passCloses #1241